ENH: Add Enumeration states to XTCE parser #783
Conversation
This adds the ability to include derived enumeration states from the "States" tab in the spreadsheets.
| @@ -329,6 +328,10 @@ def _add_parameter(self, row: pd.Series, total_packet_bits: int) -> None: | |||
| # Go look up the conversion in the AnalogConversions tab | |||
| # and add it to the encoding | |||
| self._add_analog_conversion(row, encoding) | |||
| elif row["convertAs"] == "STATE": | |||
There was a problem hiding this comment.
I didn't notice that this column and that tab was connected. I knew that they stored enums(derived) values in that tab but didn't connect those two together. Nice!
| for _, state_row in state_sheet.iterrows(): | ||
| enumeration = Et.SubElement(enumeration_list, "xtce:Enumeration") | ||
| enumeration.attrib["value"] = str(state_row["value"]) | ||
| enumeration.attrib["label"] = str(state_row["state"]) |
There was a problem hiding this comment.
I am wondering if this function produces something like this?
<xtce:EnumeratedParameterType name="QUARTER_CYCLE_ENUM" signed="false">
<xtce:IntegerDataEncoding sizeInBits="5" encoding="unsigned"/>
<xtce:EnumerationList>
<xtce:Enumeration label="FIRST" value="0"/>
<xtce:Enumeration label="SECOND" value="1"/>
<xtce:Enumeration label="THIRD" value="2"/>
<xtce:Enumeration label="FORTH" value="3"/>
</xtce:EnumerationList>
</xtce:EnumeratedParameterType>
I saw most of it but didn't see it add this line. I think we need that.
<xtce:IntegerDataEncoding sizeInBits="5" encoding="unsigned"/>
There was a problem hiding this comment.
That is taken care of in the above _add_parameter_type(), which adds the encodings. Then here we modify that parameter type to be EnumeratedParameterType instead of IntegerParameterType. It should be there if you run this on the swapi or swe definitions (could you verify it works for you as well). This is where tests would be nice...
There was a problem hiding this comment.
I ran locally and it looks correct.
greglucas
force-pushed
the
xtce-enumerations
branch
from
d9f1968
to
6b8bff9
Compare
Of course, I totally missed that we had an Excel sheet and expected output in the I've changed from that Excel sheet to a dynamically generated Excel file instead as I suggested initially. The expected output file has a minimal diff with the new enumeration updates. |
|
@bourque , I am pinging you to see if you agree with updating/changing the tests here now. Codecov is now 100% for this diff. |
greglucas
merged commit 373f9f3
into
IMAP-Science-Operations-Center:dev
Change Summary
Overview
This adds the ability to include derived enumeration states from the "States" tab in the spreadsheets.
Testing
Manual testing with SWE's packet definition. Unfortunately it looks like we don't have any tests for this utility and it is a pain to mock up the spreadsheets. This should be an action item to do in the future though.
closes #782